Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration with binsider #45

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Integration with binsider #45

wants to merge 11 commits into from

Conversation

godzie44
Copy link
Owner

@godzie44 godzie44 commented Oct 1, 2024

No description provided.

- rename `VariableSelector` into `Selector`
- rename `DqeResult` into `ScopedVariable`
- use `Selector::by_name` instead of manual creating of `Name` selector
- add reminder comment for oracle watchpoints
- rename `TypeIdentity` into `TypeId`
- add `TypeIdenty` type - name + namespace
- use type `TypeIdenty` in `VariableIR` instead of just name
…f `VariableIR` as structure members and array items representation
Copy link
Contributor

@orhun orhun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good start!

I realized some things are missing, so I might contribute next week. Mainly, hexdump keys and selecting dynamic libraries does not seem to be working.

Btw, how do you feel about this? Do you think the integration was worth it, is the API okay and so on? I'm happy to make improvements based on your feedback :)

Cargo.toml Outdated Show resolved Hide resolved
README.md Outdated
similar to readelf(1) and strace(1).
It lets you inspect strings, examine linked libraries, and perform hexdumps, all within a user-friendly TUI.

- `binsider` - run binsider inside a debugger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider also supporting a short version like b or bn since binsider might be a long to type.

Also, we can have more specific commands as shortcuts, for example:

  • static_info -> binsider --tab static-analysis
  • strings -> binsider --tab strings

And so on :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! I'm add bn shortcut. About specific commands - i think it's nice too, but looks like we need to make simple api for it first :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable!

break;
}
let command = Command::from(key);
state.run_command(command, sender.clone())?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also disable some commands here FYI - e.g. I'm not sure selecting dynamic dependencies work as expected.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I made it as simple as possible for two reasons:

  1. this is a development branch of the bs and I think its ok to have some non-working functionality for now
  2. i expect that we will be able to make all the functionality work before the next 0.x release of bs :)

@godzie44
Copy link
Owner Author

godzie44 commented Oct 6, 2024

Thank @orhun , I think current integration is a good start too, but we need to do some improvements :) I'm going to create a draft PR in binsider for your review (it should make the external API nicer to integrate)

It's sad that it's impossible to integrate BugStalker TUI into binsider now.
And what do you think about the idea of ​​trying to integrate the bs console into binsider? Maybe you know some examples of integrating a text terminal into tui applications?

btw I've been on a little trip and now I'll have more time to work on this integration and help with the development of binsider

@orhun
Copy link
Contributor

orhun commented Oct 8, 2024

I'm going to create a draft PR in binsider for your review (it should make the external API nicer to integrate)

Just left a comment there, the rationale for the changes weren't quite clear to me.

It's sad that it's impossible to integrate BugStalker TUI into binsider now.

I don't think it is impossible, it is just a bit of work.

And what do you think about the idea of ​​trying to integrate the bs console into binsider? Maybe you know some examples of integrating a text terminal into tui applications?

Yup, I think in the worst case we can spawn a terminal inside the TUI (using tui-term). However, I need to look into the API of BugStalker first to make sure, do you have any API usage examples & other apps depending on BugStalker for now?

I will look into this and try to integrate something (on stream, probably).

@godzie44
Copy link
Owner Author

godzie44 commented Oct 8, 2024

No, there is no other apps that using bs. As example i can only show this function https://github.com/godzie44/BugStalker/blob/master/src/ui/console/mod.rs#L376.

Unfortunately, this also all looks complicated :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants